The Accept: header vs. #caches_page

Posted by Mike Burns

Oct 21

In this example we're going to have an XML API on FontsController#index. A GET to /fonts.xml will produce a list of every Font in the database, along with all its information (name, thumbnail, a list of ligatures, price, license, and so on).

This is a long list. Luckily it's just for the API consumers. The normal HTML people just request /fonts and this gives them a paginated view of the lovely fonts on our system.

So to speed it up we do some simple caching. In FontsController, at the top, we add this:

1
2
3

caches_page :index, :if => Proc.new {|c|
  c.request.format.xml?
}

And magically requests to /fonts.xml are cached to public/fonts.xml. Lovely! Thanks, Rails!

The Problem

So what happens when someone requests /fonts with an Accept: text/xml header? You can try it like this:



curl -H 'Accept: text/xml' http://ihearthelvetica.local/fonts

No XMLFontsController#index uses #respond_to, so it sends back the XML as requested. However, #caches_page saves the XML to public/fonts.html! Now when a user requests /fonts from their Web browser, they're getting back a mess of XML!

No good.

The Workaround

This is a problem deep in the Rails caching code. As a workaround, try this on for size:

1
2
3
4
5
6
7
8
9
10
11

class ApplicationController < ActionController::Base
  before_filter :fix_caching_extension_for_xml

private

  def fix_caching_extension_for_xml
    if request.format.xml?
      ActionController::Base.page_cache_extension = '.xml'
    end
  end
end

This manually sets the extension for XML requests to .xml, so that it saves it to the right place.

The Fix

The other option is to fix this in Ruby on Rails itself. Download the patch attached to the ticket I've opened and apply it to an edge version of Rails. The patch has tests and is more generalized, so if the workaround fails to solve your problem the patch might.

Leave a comment on the Lighthouse ticket if the patch works for you, or if you've encountered this problem. Together, we can.

0 comments on this post

Suspenders

Posted by Dan Croak

Oct 21

Introducing Suspenders, the thoughtbot Rails template.

Install

To create a new project, checkout the github repository and run:



./script/create_project projectname

This will create a project in ../projectname. You should then follow the instructions on Github to upload that project there. This script creates an entirely new git repository, and is not meant to be used against an existing repo.

Changes to the template can be easily pulled it into your project via:



rake git:pull:suspenders

This is funny, because you’re pulling your suspenders.

About

Suspenders was created for use at thoughtbot as a baseline application setup, with reasonable default plugins that the majority (if not all) of our applications used, as well as best-practice configuration options.

Thanks to various Boston.rb people and rails rumble people for attempting to use suspenders this past weekend and giving it it’s first non-thoughtbot-internal usage.

Suspenders currently includes Rails 2.1.1

Gems (unpacked in vendor/gems):

Plugins (in vendor/plugins):

Initializers (in config/initializers)

Rake Tasks

Rake tasks are contained in the limerick_rake gem.

Testing

The basic test setup uses Test::Unit, Shoulda, factory_girl, and mocha, and includes some standard “shoulda macros” that we’ve used on various projects.

Factory Girl is a fixture replacement library, following the factory pattern. Place your factories in test/factories.rb. The fixture directory has been removed, as fixtures are not used.

Shoulda is a pragmatic testing framework for TDD and BDD built on top of Test::Unit. A number of additional testing macros are provided in test/shoulda_macros:

Deployment

Deployment is done using capistrano, and deploys to a mongrel cluster, running under Apache.

Rake tasks are provided for managing git branches which the different environments pull from for deploy.

To push the git master to git staging branch run: 


rake git:push:staging
To push the git staging branch to production branch run: 


rake git:push:production
Setup your deployment environment by running: 


cap ENVIRONMENT deploy:setup

You’ll be prompted for the environment’s database password

Deploy to the desired environment by running: 


cap ENVIRONMENT deploy
The default environment for deploy is staging, to deploy to staging, just run: 


cap deploy

Mascot

The official Suspenders mascot is Suspenders Boy


3 comments on this post

Non-test code is trivial

Posted by Mike Burns

Oct 17

Testing! Do not disturb!

I have heard—and been in the middle of—arguments which make the claim that the tests is the stupid, simple part and the "real code" is the hard part. One such person made the claim that interns should write the test code and "real programmers" should write code to make the tests pass.

This is backwards.

The tests are the program. Without the tests, the program does not work. Tests are not something that should be left for the inexperienced; tests are the hard part.

To quote Code Complete:

Test cases are often as likely or more likely to contain errors than the code being testing (Weiland 1983, Jones 1986a, Johnson 1994 [Johnson, Mark. 1994. "Dr. Boris Beizer on Software Testing: An Interview"]). The reasons are easy to find—especially when the developer writes the test case. Test cases tend to be created on the fly rather than through a careful design and construction process. They are often viewed as one-time tests and are developed with the care commensurate with something to be thrown away.

For example, the following test does not have nearly enough setup to be useful (assigns(:posts) could be empty, causing this test to be something of a no-op). This is a common problem; sometimes it happens because it relies on fixtures that get lost in a refactoring, and sometimes it happens out of just not thinking hard enough. The non-test code behind this, of course, is part of the Rails scaffold and dead simple.

1
2
3
4
5
6

context "a GET to index" do
  setup { get :index }
  should "have a link for each post" do
    assigns(:posts).each { |post| assert_select 'a[href=?]', post_path(post) }
  end
end

The next set of tests could use a loop over %w(index show edit) and should_redirect_to to shorten it without any loss of knowledge (and could even make it more clear). This is a case of an accurate series of tests (for a one-line piece of code) that has become difficult to maintain because the developer failed to notice the pattern.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23

context "logged out" do
  setup { session[:user_id] = nil }
  context "GET to index" do
    setup { get :index }
    should "redirect to the root_url" do
      assert_redirected_to root_url
    end
  end

  context "GET to show" do
    setup { get :show }
    should "redirect to the root_url" do
      assert_redirected_to root_url
    end
  end

  context "GET to edit" do
    setup { get :edit }
    should "redirect to the root_url" do
      assert_redirected_to root_url
    end
  end
end

It's interesting to note that the problem in the first example could be caused by refactoring, and the problem in the second example is caused by not refactoring. This is a fair point, and another indication of the difficulty in writing tests: when refactoring non-test code, anything you break is pointed out quickly by the tests; when refactoring test code, you need to make sure that it will continue to fail when needed, and continue to pass when needed.

Some tests are just so arcane or trivial that they would normally be overlooked, but if it really needs to be a specific way then it needs to be tested.

For example, ActiveRecord::Base#to_xml is a lovely part of Rails that is often used for part of the application's public API. If #to_xml changes, many customers will be upset. Recently we had to override #to_xml with lots of special-casing; while the special-casing did get tested, some of the more subtle parts of #to_xml were ignored. Thus, this test was recently added in response to a bug report:

1
2
3
4

should "dasherize any element with underscores when sent #to_xml" do
  xml = Factory(:message).to_xml
  assert_no_match /<\w*_\w*>/, xml
end

Some code needs a complex maze of tests. Advanced search is a common feature that requires careful thought to test all the options. The Album.advanced_search method may be large and complicated, but the tests for it must be necessarily more so. No time for laziness or ignorance here; this test suite requires the knowledge of an advanced search expert, and the patience of a person who has a lot of patience.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

context "when sent #advanced_search" do
  setup { create_all_sorts_of_data }
  %w(producer artist mixing_engineer).each do |str_opt|
    context "with the #{str_opt} option" do
      # ...
    end
  end

  [true,false].each do |include_compilation|
    context "with :include_compilations option set to #{include_compilation}" do
      # ...
    end
  end

  # ... and then permutations of these, too
end

(While researching this post I found these two interesting links: a WardsWiki page about bugs in tests, in which people argue over buggy tests, refactoring tests, and how this affects non-test code; and a paper titled Refactoring Test Code [PDF], which discusses "code smells" relevant to testing and how best to refactor these.)

Known fact: chicks are totally into dudes who write awesome tests.


14 comments on this post

© 2000 - 2009 by thoughtbot, inc.
written by a bushel of tiny robots